Skip to content

put http and https modules behind compat flag #4456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 15, 2025
Merged

put http and https modules behind compat flag #4456

merged 2 commits into from
Jul 15, 2025

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Jul 3, 2025 •
edited
Loading

This pull-request enables the following modules and functions behind a compat flag

  • node:_http_agent
    • Agent
    • globalAgent
  • node:_http_client
    • ClientRequest
  • node:_http_common
    • _checkIsHttpToken
    • _checkInvalidHeaderChar
    • chunkExpression
    • continueExpression
    • methods
    • kIncomingMessage
  • node:_http_incoming
    • IncomingMessage
  • node:_http_outgoing
    • validateHeaderName
    • validateHeaderValue
    • kUniqueHeaders
    • kHighWaterMark
    • parseUniqueHeadersOption
    • OutgoingMessage
  • node:http
    • validateHeaderName
    • validateHeaderValue
    • METHODS
    • STATUS_CODES
    • ClientRequest
    • request
    • get
    • WebSocket
    • OutgoingMessage
    • Agent
    • globalAgent
    • IncomingMessage
  • node:https
    • get
    • request
    • Agent
    • globalAgent

cc @vicb

@anonrig anonrig requested review from guybedford, jasnell and npaun July 3, 2025 18:43
@anonrig anonrig requested review from a team as code owners July 3, 2025 18:43
@vicb
Copy link
Contributor

vicb commented Jul 3, 2025 •
edited
Loading

Thanks for the listing the changes, it will help syncing the unenv preset

A few questions first:

  • what are the timeline to merge this PR?
  • The change was merged 2 days ago, how has it been validated since then ? (i.e. why is now a good time to remove the exp flag, did it reach prod already?)
  • Has a doc PR/issue been created to update https://developers.cloudflare.com/workers/runtime-apis/nodejs/?
  • When I asked earlier about Agent support there were some limitations, have those been lifted or still apply? If they still apply it would be nice to add them to the PR description - as well as any other diff with the Node API.

But then, I'd like to thank you for the implementation. It has been requested many times by users and it should really improve the compatibility with Node 🙏

I'll work on updating the unenv preset as soon as this is released.

@jasnell
Copy link
Collaborator

jasnell commented Jul 3, 2025

what are the timeline to merge this PR?

I think we can merge this any time really. The big question is when should we unflag in production.

The #4205 was merged 2 days ago, how has it been validated since then ? (i.e. why is now a good time to remove the exp flag, did it reach prod already?)

Before we flip the switch to make this generally available in production, we should have some internal teams do some experimentation to help validate this. I don't think we've done so yet.

Has a doc PR/issue been created to update developers.cloudflare.com/workers/runtime-apis/nodejs?

I've not yet seen a documentation PR opened. We'll definitely need one... https://github.com/cloudflare/cloudflare-docs/pulls?q=sort%3Aupdated-desc+is%3Apr+is%3Aopen+%22node%3Ahttp%22+

When I asked earlier about Agent support there were some limitations, have those been lifted or still apply? If they still apply it would be nice to add them to the PR description - as well as any other diff with the Node API.

Pretty sure the restrictions are still there. We're not really implementing Agent. What is there is a largely non-functional stub. We should absolutely make sure these make it into the docs.

Personally, I'd like to at least see the documentation PR opened before we merge this.

@codspeed-hq

This comment was marked as resolved.

Sign in to view
@anonrig anonrig mentioned this pull request Jul 7, 2025
Merged
@anonrig
Copy link
Member Author

anonrig commented Jul 7, 2025 •
edited by vicb
Loading

Opened a PR for docs @jasnell @vicb #4456

cloudflare/cloudflare-docs#23481

@vicb vicb mentioned this pull request Jul 8, 2025
Closed
2 tasks
jasnell
jasnell approved these changes Jul 9, 2025
View reviewed changes
Copy link
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I'd prefer to hold off on merging this until after it hits production and internal teams have a chance to experiment with it a bit.

@vicb

This comment was marked as resolved.

Sign in to view
@anonrig anonrig force-pushed the yagiz/remove-experimental branch from 70bea86 to 08742cb Compare July 15, 2025 17:10
@anonrig anonrig changed the title remove experimental flag from http and https modules put http and https modules behind compat flag Jul 15, 2025
@anonrig anonrig force-pushed the yagiz/remove-experimental branch from 08742cb to 8afb38f Compare July 15, 2025 17:12
danlapid
danlapid reviewed Jul 15, 2025
View reviewed changes
@anonrig anonrig mentioned this pull request Jul 15, 2025
Closed
@anonrig anonrig merged commit c931b08 into main Jul 15, 2025
20 of 21 checks passed
@anonrig anonrig deleted the yagiz/remove-experimental branch July 15, 2025 20:20
joshthoward pushed a commit that referenced this pull request Aug 7, 2025
@anonrig @joshthoward
put http and https modules behind compat flag (#4456)
1938023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danlapid danlapid danlapid left review comments

@jasnell jasnell jasnell approved these changes

@npaun npaun npaun approved these changes

@guybedford guybedford Awaiting requested review from guybedford

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@anonrig @vicb @jasnell @npaun @danlapid